Skip to content

Conversation

@viliml
Copy link

@viliml viliml commented Sep 28, 2024

Made it recognize std::ops::Add::add and std::ops::Mul::mul, as well as prevented it from adding a turbofish when it is the return value of a function with a known return type.

changelog: none

@rustbot
Copy link
Collaborator

rustbot commented Sep 28, 2024

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Centri3 (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Sep 28, 2024
Copy link
Member

@Centri3 Centri3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple nits, thanks!


let turbofish = if replacement.has_generic_return {
format!("::<{}>", cx.typeck_results().expr_ty_adjusted(right_expr).peel_refs())
format!("::<{}>", cx.typeck_results().expr_ty(expr))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this change make any difference? More specifically was the original code unnecessary

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I did it to be consistent with the new function I made, where there is no right_expr. And it is more straightforward and sure to work, as the type argument to sum and product is exactly the return type, which is the type of the whole expression.

if let hir::Node::Block(block) = parent
&& let grandparent = cx.tcx.parent_hir_node(block.hir_id)
&& let hir::Node::Expr(grandparent_expr) = grandparent
&& let ggparent = cx.tcx.parent_hir_node(grandparent_expr.hir_id)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably better to iterate over the parents? is there any reason simply getting the containing item doesn't work?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know, I don't understand this infrastructure so well so my first try was probably not the best way to do it.

@Centri3
Copy link
Member

Centri3 commented Dec 2, 2024

Hey @viliml, been almost a month and I'm pinging from triage. Has there been any progress? The implementation looks great to me beyond some small changes. Is there anything you're stuck on or have any questions?

@Centri3
Copy link
Member

Centri3 commented Feb 15, 2025

Hey @viliml, another ping. The last message still applies but at this point, if you don't have the time, is it ok/preferred if I push to your branch any nits I have said or find?

@Centri3
Copy link
Member

Centri3 commented Feb 15, 2025

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Feb 15, 2025
@rustbot
Copy link
Collaborator

rustbot commented Mar 31, 2025

☔ The latest upstream changes (possibly d28d234) made this pull request unmergeable. Please resolve the merge conflicts.

@nyabinary
Copy link

bumping @viliml again :3

@viliml
Copy link
Author

viliml commented Sep 4, 2025

Oh, I'm sorry for not responding to your pings. I did this mostly just for fun and to see how Clippy works from the inside, so I kinda forgot about it afterwards and didn't check my notifications.

is it ok/preferred if I push to your branch any nits I have said or find?

Yes, it's OK, that's why I checked the "allow commits by maintainers" option.

@Jarcho
Copy link
Contributor

Jarcho commented Sep 17, 2025

@Centri3 are you going to push this through? Sounds like @viliml doesn't plan to continue this.

@Jarcho Jarcho closed this Oct 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants